-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ABI flags in RISC-V/LoongArch ELF file generated by rustc #114332
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -79,6 +79,14 @@ LL | let (A(A(..) | B(a), _) | B(A(a, _) | B(a))) = Y; | |||
| | | |||
| pattern doesn't bind `a` | |||
|
|||
error[E0408]: variable `c` is not bound in all patterns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happened here... I assume this is related to adding c
as a known symbol???
Yeah, I renamed c
to builtin_syntax
and the error changed in nightly. Looks like it's sorting the symbols by their index? That's strange and bad, but luckily not your problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I see, I'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bors r+ |
📌 Commit 8c53a72df236240bb8926ecf52c39aee4c628640 has been approved by It is now in the queue for this repository. |
⌛ Testing commit 8c53a72df236240bb8926ecf52c39aee4c628640 with merge ee51c76c691b1bb00208115da9321d0422b8b1ee... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Some of the RISC-V targets don't have llvm_abiname set explicitly. I now default unset llvm_abiname to be the same as ilp32/lp64, i.e. not using hardware floating point. |
☔ The latest upstream changes (presumably #114569) made this pull request unmergeable. Please resolve the merge conflicts. |
sorry for the late response, I've been away |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (484cb4e): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 635.189s -> 635.452s (0.04%) |
Replace `HashMap` with `IndexMap` in pattern binding resolve fixes rust-lang#114332 (comment)
Replace `HashMap` with `IndexMap` in pattern binding resolve fixes rust-lang#114332 (comment)
Replace `HashMap` with `IndexMap` in pattern binding resolve fixes rust-lang#114332 (comment)
Rollup merge of rust-lang#114454 - Nilstrieb:no-evil-sorting, r=cjgillot Replace `HashMap` with `IndexMap` in pattern binding resolve fixes rust-lang#114332 (comment)
Fix #114153
It turns out the current way to set these flags are completely wrong. In LLVM the target ABI is used instead of target features to determine these flags.
Not sure how to write a test though. Or maybe a test isn't necessary because this affects only those touching target json?
r? @Nilstrieb